Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Pods targets inherit deployment target from the app when higher #23679

Closed
wants to merge 2 commits into from

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Oct 18, 2024

I while working on #23678, noticed a few warnings like:

The iOS deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 11.0, but the range of supported deployment target versions is 12.0 to 18.0.99.

Screenshot 2024-10-18 at 3 50 54 PM

This post install hook makes pods targets that have a deployment target lower than the app target adopt the app deployment target.

Before

image

After

image

Regression Notes

  1. Potential unintended areas of impact – N.A.
  2. What I did to test those areas of impact (or what existing automated tests I relied on) – N.A.
  3. What automated tests I added (or what prevented me from doing so) – N.A.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes. N.A.
  • I have considered adding accessibility improvements for my changes. N.A.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

UI changes testing checklist: Not a UI PR.

I noticed a few warnings like:

  The iOS deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 11.0,
  but the range of supported deployment target versions is 12.0 to
  18.0.99.

This post install hook makes pods targets that have a deployment target
lower than the app target adopt the app deployment target.
Comment on lines +115 to +118
# Let Pods targets inherit deployment target from the app
# This solution is suggested here: https://github.com/CocoaPods/CocoaPods/issues/4859
pod_ios_deployment_target = Gem::Version.new(config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'])
config.build_settings.delete 'IPHONEOS_DEPLOYMENT_TARGET' if pod_ios_deployment_target <= APP_IOS_DEPLOYMENT_TARGET
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting that the warning Xcode gives is that the deployment target is set to a version lower than the minimum supported one. An alternative fix would have been to set the target to the minimum supported version. The solution here is more portable, though, because it doesn't require updating the override every time the minimum supported target changes.

@mokagio mokagio requested review from crazytonyli and kean October 18, 2024 05:04
@mokagio mokagio added the Tooling Build, Release, and Validation Tools label Oct 18, 2024
@mokagio mokagio added this to the 25.5 milestone Oct 18, 2024
@mokagio mokagio enabled auto-merge October 18, 2024 05:04
@mokagio mokagio marked this pull request as draft October 18, 2024 05:50
auto-merge was automatically disabled October 18, 2024 05:50

Pull request was converted to draft

@mokagio
Copy link
Contributor Author

mokagio commented Oct 18, 2024

Reverted to draft because of the CI failures...

@dangermattic
Copy link
Collaborator

1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

Copy link
Contributor

@kean kean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@jkmassel
Copy link
Contributor

I did a worse version of this in #23736 that should carry us through until we're ready to drop CocoaPods entirely.

@jkmassel jkmassel closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tooling Build, Release, and Validation Tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants